Support validating keyless signed images in v-e-c tekton task by simonbaird · Pull Request #3140 · conforma/cli

simonbaird · 2026-02-28T20:12:15Z

See commit messages for explanations.

Ref: https://issues.redhat.com/browse/EC-1652

qodo-code-review · 2026-02-28T20:12:33Z

Review Summary by Qodo

Add keyless signing verification support to verify-enterprise-contract task

✨ Enhancement 🧪 Tests

Walkthroughs

Description

• Add keyless signing verification support to verify-enterprise-contract task
  - New CERTIFICATE_IDENTITY and CERTIFICATE_OIDC_ISSUER parameters
  - Conditional logic to use keyless or public-key verification
• Convert task validate step from command to script format
  - Enables flexible parameter handling for keyless signatures
• Create test infrastructure for keyless signed images
  - Scripts to generate and verify test images with cosign v2 and v3
• Add acceptance tests for keyless signature verification
  - Tests for both cosign v2 and v3 signature formats

Diagram

flowchart LR
  A["verify-enterprise-contract Task"] -->|"Add Parameters"| B["CERTIFICATE_IDENTITY<br/>CERTIFICATE_OIDC_ISSUER"]
  A -->|"Convert to Script"| C["Flexible Parameter<br/>Handling"]
  C -->|"Support"| D["Keyless Verification"]
  C -->|"Support"| E["Public-Key Verification"]
  F["Test Image Creation"] -->|"Generate"| G["Keyless Signed Images<br/>v2 and v3"]
  G -->|"Verify in"| H["Acceptance Tests"]

File Changes

1. tasks/verify-enterprise-contract/0.1/verify-enterprise-contract.yaml ✨ Enhancement +62/-42

Add keyless verification parameters and convert to script

• Added two new parameters CERTIFICATE_IDENTITY and CERTIFICATE_OIDC_ISSUER for keyless
 verification
• Converted validate step from command format to bash script format
• Implemented conditional logic to choose between keyless and public-key verification based on
 provided parameters
• Maintained all existing parameters and functionality while adding new capability

tasks/verify-enterprise-contract/0.1/verify-enterprise-contract.yaml

2. docs/modules/ROOT/pages/verify-enterprise-contract.adoc 📝 Documentation +2/-0

Document keyless verification parameters
• Added documentation for two new parameters: CERTIFICATE_IDENTITY and CERTIFICATE_OIDC_ISSUER
• Explained purpose of each parameter for keyless verification workflow
docs/modules/ROOT/pages/verify-enterprise-contract.adoc

3. hack/keyless-test-image/create.sh 🧪 Tests +76/-0

Create keyless signed test images script

• Script to create and sign test images with cosign v2 and v3
• Builds minimal UBI-based container images
• Signs images keylessly and creates SLSA provenance attestations
• Creates two versions to test different cosign signature formats

hack/keyless-test-image/create.sh

View more (3)

4. hack/keyless-test-image/helpers.sh 🧪 Tests +27/-0

Helper functions for test image scripts

• Utility functions for test image creation scripts
• Provides formatted output functions (h1, pause, nl)
• Supports interactive script execution with user prompts

hack/keyless-test-image/helpers.sh

5. hack/keyless-test-image/verify.sh 🧪 Tests +75/-0

Verify keyless signed test images script

• Script to verify keyless signed test images
• Tests cosign v2 and v3 verification commands
• Demonstrates backwards and forwards compatibility between cosign versions
• Uses personal identity for verification (sbaird@redhat.com with Google OIDC)

hack/keyless-test-image/verify.sh

6. features/task_validate_image.feature 🧪 Tests +78/-0

Add keyless signature acceptance tests

• Added two new acceptance test scenarios for keyless signing verification
• Test for cosign v2 style signatures with expected success
• Test for cosign v3 style signatures with expected failure (pending upstream fix)
• Uses external test images from quay.io/conforma/test registry
• Includes certificate identity and OIDC issuer parameters in test configuration

features/task_validate_image.feature

qodo-code-review · 2026-02-28T20:12:34Z

Code Review by Qodo

🐞 Bugs (5) 📘 Rule violations (0) 📎 Requirement gaps (0)

1. Tekton script param injection 🐞 Bug ⛨ Security

Description

The validate step now inlines $(params.*) into a bash script; parameter values containing
quotes/newlines (e.g., JSON policy configs) can break the script or be exploited for shell
injection, and unquoted --flag=$(params.X) entries can be word-split by bash.

Code

tasks/verify-enterprise-contract/0.1/verify-enterprise-contract.yaml[R253-300]

+      script: |
+        #!/bin/bash
+        set -euo pipefail
+
+        cmd_args=(
+          "validate"
+          "image"
+          "--images" "/tekton/home/snapshot.json"
+          "--policy" "$(params.POLICY_CONFIGURATION)"
+        )
+
+        if [ -n "$(params.CERTIFICATE_IDENTITY)" ] && [ -n "$(params.CERTIFICATE_OIDC_ISSUER)" ]; then
+          cmd_args+=(
+            "--certificate-identity" "$(params.CERTIFICATE_IDENTITY)"
+            "--certificate-oidc-issuer" "$(params.CERTIFICATE_OIDC_ISSUER)"
+          )
+        elif [ -n "$(params.PUBLIC_KEY)" ]; then
+          cmd_args+=(
+            "--public-key" "$(params.PUBLIC_KEY)"
+          )
+        fi
+
+        cmd_args+=(
+          "--rekor-url" "$(params.REKOR_HOST)"
+          "--ignore-rekor=$(params.IGNORE_REKOR)"
+          "--workers" "$(params.WORKERS)"
+          # NOTE: The syntax below is required to negate boolean parameters
+          "--info=$(params.INFO)"
+          # Fresh versions of ec support "--timeout=0" to indicate no timeout, but this would break
+          # the task if it's used with an older version of ec. In an abundance of caution, let's set
+          # an arbitrary high value instead of using 0 here. In future we can change it to 0.
+          # (The reason to not use an explicit timeout for ec is so Tekton can handle the timeouts).
+          "--timeout=100h"
+          "--strict=false"
+          "--show-successes"
+          "--effective-time=$(params.EFFECTIVE_TIME)"
+          "--extra-rule-data=$(params.EXTRA_RULE_DATA)"
+          "--retry-max-wait" "$(params.RETRY_MAX_WAIT)"
+          "--retry-max-retry" "$(params.RETRY_MAX_RETRY)"
+          "--retry-duration" "$(params.RETRY_DURATION)"
+          "--retry-factor" "$(params.RETRY_FACTOR)"
+          "--retry-jitter" "$(params.RETRY_JITTER)"
+          "--output" "text=$(params.HOMEDIR)/text-report.txt?show-successes=false"
+          "--output" "appstudio=$(results.TEST_OUTPUT.path)"
+          "--output" "json=$(params.HOMEDIR)/report-json.json"
+        )
+
+        exec ec "${cmd_args[@]}"

Evidence
The validate step embeds Tekton param substitutions directly into the bash script, including
"--policy" "$(params.POLICY_CONFIGURATION)". Another task in this repo explicitly avoids this
pattern by passing POLICY_CONFIGURATION via env to avoid shell quoting issues, and repo acceptance
tests show POLICY_CONFIGURATION can indeed be a JSON string with embedded quotes/backslashes.
tasks/verify-enterprise-contract/0.1/verify-enterprise-contract.yaml[253-300]
tasks/verify-conforma-konflux-ta/0.1/verify-conforma-konflux-ta.yaml[291-346]
features/ta_task_validate_image.feature[65-73]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The `validate` step uses a Tekton `script:` that directly interpolates `$(params.*)` into bash. If a param contains quotes/newlines (notably `POLICY_CONFIGURATION` when supplied as JSON), it can break the script or allow shell injection. Several `--flag=$(params.X)` arguments are also unquoted and can be word-split.
### Issue Context
This repo already documents this exact problem and solves it in `verify-conforma-konflux-ta` by passing `POLICY_CONFIGURATION` via environment variables.
### Fix Focus Areas
- tasks/verify-enterprise-contract/0.1/verify-enterprise-contract.yaml[253-310]
### Suggested approach
- Add env vars for values used in the script (at least `POLICY_CONFIGURATION`, `PUBLIC_KEY`, `CERTIFICATE_IDENTITY`, `CERTIFICATE_OIDC_ISSUER`, and any other user-controlled strings).
- In the script, reference variables (e.g. `&amp;quot;$POLICY_CONFIGURATION&amp;quot;`) and ensure any `--flag=value` entries are wrapped/constructed safely (e.g. `&amp;quot;--extra-rule-data=${EXTRA_RULE_DATA}&amp;quot;`).
- Avoid placing raw `$(params.X)` inside the script body wherever possible.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. Task README not updated 🐞 Bug ✧ Quality ⭐ New

Description

The task YAML adds CERTIFICATE_IDENTITY and CERTIFICATE_OIDC_ISSUER, but the task’s own README.md
doesn’t document these new parameters, reducing discoverability for users installing the task from
that README. This creates documentation drift within the repo (docs are updated elsewhere, but task
README remains incomplete).

Code

tasks/verify-enterprise-contract/0.1/verify-enterprise-contract.yaml[R72-85]

+    - name: CERTIFICATE_IDENTITY
+      type: string
+      description: >-
+        Expected identity in the signing certificate for keyless verification.
+        This should be the email or URI that was used when signing.
+      default: ""
+
+    - name: CERTIFICATE_OIDC_ISSUER
+      type: string
+      description: >-
+        Expected OIDC issuer in the signing certificate for keyless verification.
+        This should match the issuer that provided the identity token used for signing.
+      default: ""
+

Evidence
The task definition includes two new optional params for keyless verification, but the task-level
README’s optional parameter list omits them, so users following that README won’t learn how to
configure keyless verification via the task params.
tasks/verify-enterprise-contract/0.1/verify-enterprise-contract.yaml[72-84]
tasks/verify-enterprise-contract/0.1/README.md[29-51]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The task-level README.md does not mention the new keyless verification parameters, so users relying on this README won’t know how to use the feature.

### Issue Context
The AsciiDoc docs were updated, but the task directory README is still commonly used for installation and parameter discovery.

### Fix Focus Areas
- tasks/verify-enterprise-contract/0.1/README.md[29-51]
- tasks/verify-enterprise-contract/0.1/verify-enterprise-contract.yaml[72-84]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. Partial keyless config ignored 🐞 Bug ✓ Correctness

Description

Keyless verification is enabled only when BOTH CERTIFICATE_IDENTITY and CERTIFICATE_OIDC_ISSUER are
set; if a user sets only one, the task silently ignores it (and may fall back to PUBLIC_KEY or
neither), causing unexpected verification behavior.

Code

tasks/verify-enterprise-contract/0.1/verify-enterprise-contract.yaml[R264-273]

+        if [ -n "$(params.CERTIFICATE_IDENTITY)" ] && [ -n "$(params.CERTIFICATE_OIDC_ISSUER)" ]; then
+          cmd_args+=(
+            "--certificate-identity" "$(params.CERTIFICATE_IDENTITY)"
+            "--certificate-oidc-issuer" "$(params.CERTIFICATE_OIDC_ISSUER)"
+          )
+        elif [ -n "$(params.PUBLIC_KEY)" ]; then
+          cmd_args+=(
+            "--public-key" "$(params.PUBLIC_KEY)"
+          )
+        fi

Evidence
The script checks both keyless params with an AND; there is no validation/error when only one is
provided. A similar task in this repo fails fast on incomplete paired configuration (showing the
repo’s preferred UX for misconfiguration).
tasks/verify-enterprise-contract/0.1/verify-enterprise-contract.yaml[264-273]
tasks/verify-enterprise-contract/0.1/verify-enterprise-contract.yaml[72-84]
tasks/verify-conforma-konflux-ta/0.1/verify-conforma-konflux-ta.yaml[325-329]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The task currently ignores partial keyless configuration (only identity or only issuer). This can lead to confusing behavior and potentially running verification with unintended settings.
### Issue Context
The keyless flags are logically paired (identity + issuer). The repo already uses fail-fast validation for incomplete paired config in other tasks.
### Fix Focus Areas
- tasks/verify-enterprise-contract/0.1/verify-enterprise-contract.yaml[264-273]
### Suggested approach
- In the script:
- If exactly one of `CERTIFICATE_IDENTITY` / `CERTIFICATE_OIDC_ISSUER` is set: print an ERROR and `exit 1`.
- If both are set and `PUBLIC_KEY` is also set: either error out (preferred to avoid ambiguity) or log which mode wins.
- Keep current behavior when neither keyless param is set (allow policy-provided key config).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

4. Keyless test is fragile 🐞 Bug ⛯ Reliability

Description

New acceptance tests depend on an externally hosted image and a hard-coded personal signing
identity, which may break if the image is rebuilt/removed or re-signed under a different identity.

Code

features/task_validate_image.feature[R362-375]

+    # See hack/keyless-test-image for how the test image was created. It's not ideal
+    # that this test requires an external image, but we already do this elsewhere, so
+    # I guess one more is okay. I'm hard coding the identity used to sign the image
+    # which is my personal account. That might have to change if the image is recreated.
+    #
+    # Todo: We should be able test this also with an internal image similar to how it's
+    # done in the "happy day with keyless" scenario in validate_image.feature.
+    #
+    When version 0.1 of the task named "verify-enterprise-contract" is run with parameters:
+      | IMAGES                  | {"components": [{"containerImage": "quay.io/conforma/test:keyless_v2@sha256:2dbc250c79306c30801216e37cd25164c64fda9ac3b9677c5eb0860cb13dbb87"}]} |
+      | POLICY_CONFIGURATION    | ${NAMESPACE}/${POLICY_NAME}                                               |
+      | CERTIFICATE_IDENTITY    | sbaird@redhat.com                                                         |
+      | CERTIFICATE_OIDC_ISSUER | https://accounts.google.com                                               |
+      | REKOR_HOST              | https://rekor.sigstore.dev                                                |

Evidence
The scenario explicitly calls out the external dependency and that the identity is a personal
account, and the helper scripts also bake in that identity/issuer. This is a known maintenance risk
for CI stability.
features/task_validate_image.feature[362-376]
hack/keyless-test-image/verify.sh[24-29]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Acceptance tests rely on an external quay.io image signed with a personal identity. This is brittle and can cause unrelated CI failures if the image/identity changes.
### Issue Context
The scenario itself acknowledges this risk.
### Fix Focus Areas
- features/task_validate_image.feature[341-381]
- hack/keyless-test-image/create.sh[22-33]
- hack/keyless-test-image/verify.sh[24-33]
### Suggested approach
- Replace the external fixture with a test image created/signed during the test run (if possible), or
- Move the fixture to a project-controlled repository and sign with a stable service identity; document rotation procedure.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

5. Hack scripts non-reproducible 🐞 Bug ⛯ Reliability ⭐ New

Description

The new hack scripts run cosign via go run ...@latest, making reproduction non-deterministic and
harder to audit when recreating the test images. This is low-risk because it’s under hack/, but
pinning versions would improve repeatability.

Code

hack/keyless-test-image/create.sh[R43-45]

+for ver in v2 v3; do
+  COSIGN="go run github.com/sigstore/cosign/$ver/cmd/cosign@latest"
+  LABEL="keyless_$ver"

Evidence

Using @latest means different developers (or the same developer at different times) may run
different cosign versions when recreating artifacts, producing inconsistent signatures/attestations
and making investigations harder.

hack/keyless-test-image/create.sh[43-45]
hack/keyless-test-image/verify.sh[36-38]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`go run ...@latest` is non-deterministic for recreating the keyless test images.

### Issue Context
These scripts are developer tooling (hack/), so this is not a production blocker, but pinning improves repeatability.

### Fix Focus Areas
- hack/keyless-test-image/create.sh[43-62]
- hack/keyless-test-image/verify.sh[36-63]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

ⓘ The new review experience is currently in Beta. Learn more

Previous review results

Review updated until commit 125d6d0

Results up to commit 3f4a73d

🐞 Bugs (3) 📘 Rule violations (0) 📎 Requirement gaps (0)

1. Tekton script param injection 🐞 Bug ⛨ Security

Description

The validate step now inlines $(params.*) into a bash script; parameter values containing
quotes/newlines (e.g., JSON policy configs) can break the script or be exploited for shell
injection, and unquoted --flag=$(params.X) entries can be word-split by bash.

Code

tasks/verify-enterprise-contract/0.1/verify-enterprise-contract.yaml[R253-300]

+      script: |
+        #!/bin/bash
+        set -euo pipefail
+
+        cmd_args=(
+          "validate"
+          "image"
+          "--images" "/tekton/home/snapshot.json"
+          "--policy" "$(params.POLICY_CONFIGURATION)"
+        )
+
+        if [ -n "$(params.CERTIFICATE_IDENTITY)" ] && [ -n "$(params.CERTIFICATE_OIDC_ISSUER)" ]; then
+          cmd_args+=(
+            "--certificate-identity" "$(params.CERTIFICATE_IDENTITY)"
+            "--certificate-oidc-issuer" "$(params.CERTIFICATE_OIDC_ISSUER)"
+          )
+        elif [ -n "$(params.PUBLIC_KEY)" ]; then
+          cmd_args+=(
+            "--public-key" "$(params.PUBLIC_KEY)"
+          )
+        fi
+
+        cmd_args+=(
+          "--rekor-url" "$(params.REKOR_HOST)"
+          "--ignore-rekor=$(params.IGNORE_REKOR)"
+          "--workers" "$(params.WORKERS)"
+          # NOTE: The syntax below is required to negate boolean parameters
+          "--info=$(params.INFO)"
+          # Fresh versions of ec support "--timeout=0" to indicate no timeout, but this would break
+          # the task if it's used with an older version of ec. In an abundance of caution, let's set
+          # an arbitrary high value instead of using 0 here. In future we can change it to 0.
+          # (The reason to not use an explicit timeout for ec is so Tekton can handle the timeouts).
+          "--timeout=100h"
+          "--strict=false"
+          "--show-successes"
+          "--effective-time=$(params.EFFECTIVE_TIME)"
+          "--extra-rule-data=$(params.EXTRA_RULE_DATA)"
+          "--retry-max-wait" "$(params.RETRY_MAX_WAIT)"
+          "--retry-max-retry" "$(params.RETRY_MAX_RETRY)"
+          "--retry-duration" "$(params.RETRY_DURATION)"
+          "--retry-factor" "$(params.RETRY_FACTOR)"
+          "--retry-jitter" "$(params.RETRY_JITTER)"
+          "--output" "text=$(params.HOMEDIR)/text-report.txt?show-successes=false"
+          "--output" "appstudio=$(results.TEST_OUTPUT.path)"
+          "--output" "json=$(params.HOMEDIR)/report-json.json"
+        )
+
+        exec ec "${cmd_args[@]}"

Evidence
The validate step embeds Tekton param substitutions directly into the bash script, including
"--policy" "$(params.POLICY_CONFIGURATION)". Another task in this repo explicitly avoids this
pattern by passing POLICY_CONFIGURATION via env to avoid shell quoting issues, and repo acceptance
tests show POLICY_CONFIGURATION can indeed be a JSON string with embedded quotes/backslashes.
tasks/verify-enterprise-contract/0.1/verify-enterprise-contract.yaml[253-300]
tasks/verify-conforma-konflux-ta/0.1/verify-conforma-konflux-ta.yaml[291-346]
features/ta_task_validate_image.feature[65-73]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The `validate` step uses a Tekton `script:` that directly interpolates `$(params.*)` into bash. If a param contains quotes/newlines (notably `POLICY_CONFIGURATION` when supplied as JSON), it can break the script or allow shell injection. Several `--flag=$(params.X)` arguments are also unquoted and can be word-split.

### Issue Context
This repo already documents this exact problem and solves it in `verify-conforma-konflux-ta` by passing `POLICY_CONFIGURATION` via environment variables.

### Fix Focus Areas
- tasks/verify-enterprise-contract/0.1/verify-enterprise-contract.yaml[253-310]

### Suggested approach
- Add env vars for values used in the script (at least `POLICY_CONFIGURATION`, `PUBLIC_KEY`, `CERTIFICATE_IDENTITY`, `CERTIFICATE_OIDC_ISSUER`, and any other user-controlled strings).
- In the script, reference variables (e.g. `&quot;$POLICY_CONFIGURATION&quot;`) and ensure any `--flag=value` entries are wrapped/constructed safely (e.g. `&quot;--extra-rule-data=${EXTRA_RULE_DATA}&quot;`).
- Avoid placing raw `$(params.X)` inside the script body wherever possible.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. Partial keyless config ignored 🐞 Bug ✓ Correctness

Description

Keyless verification is enabled only when BOTH CERTIFICATE_IDENTITY and CERTIFICATE_OIDC_ISSUER are
set; if a user sets only one, the task silently ignores it (and may fall back to PUBLIC_KEY or
neither), causing unexpected verification behavior.

Code

tasks/verify-enterprise-contract/0.1/verify-enterprise-contract.yaml[R264-273]

+        if [ -n "$(params.CERTIFICATE_IDENTITY)" ] && [ -n "$(params.CERTIFICATE_OIDC_ISSUER)" ]; then
+          cmd_args+=(
+            "--certificate-identity" "$(params.CERTIFICATE_IDENTITY)"
+            "--certificate-oidc-issuer" "$(params.CERTIFICATE_OIDC_ISSUER)"
+          )
+        elif [ -n "$(params.PUBLIC_KEY)" ]; then
+          cmd_args+=(
+            "--public-key" "$(params.PUBLIC_KEY)"
+          )
+        fi

Evidence
The script checks both keyless params with an AND; there is no validation/error when only one is
provided. A similar task in this repo fails fast on incomplete paired configuration (showing the
repo’s preferred UX for misconfiguration).
tasks/verify-enterprise-contract/0.1/verify-enterprise-contract.yaml[264-273]
tasks/verify-enterprise-contract/0.1/verify-enterprise-contract.yaml[72-84]
tasks/verify-conforma-konflux-ta/0.1/verify-conforma-konflux-ta.yaml[325-329]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The task currently ignores partial keyless configuration (only identity or only issuer). This can lead to confusing behavior and potentially running verification with unintended settings.

### Issue Context
The keyless flags are logically paired (identity + issuer). The repo already uses fail-fast validation for incomplete paired config in other tasks.

### Fix Focus Areas
- tasks/verify-enterprise-contract/0.1/verify-enterprise-contract.yaml[264-273]

### Suggested approach
- In the script:
 - If exactly one of `CERTIFICATE_IDENTITY` / `CERTIFICATE_OIDC_ISSUER` is set: print an ERROR and `exit 1`.
 - If both are set and `PUBLIC_KEY` is also set: either error out (preferred to avoid ambiguity) or log which mode wins.
 - Keep current behavior when neither keyless param is set (allow policy-provided key config).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. Keyless test is fragile 🐞 Bug ⛯ Reliability

Description

New acceptance tests depend on an externally hosted image and a hard-coded personal signing
identity, which may break if the image is rebuilt/removed or re-signed under a different identity.

Code

features/task_validate_image.feature[R362-375]

+    # See hack/keyless-test-image for how the test image was created. It's not ideal
+    # that this test requires an external image, but we already do this elsewhere, so
+    # I guess one more is okay. I'm hard coding the identity used to sign the image
+    # which is my personal account. That might have to change if the image is recreated.
+    #
+    # Todo: We should be able test this also with an internal image similar to how it's
+    # done in the "happy day with keyless" scenario in validate_image.feature.
+    #
+    When version 0.1 of the task named "verify-enterprise-contract" is run with parameters:
+      | IMAGES                  | {"components": [{"containerImage": "quay.io/conforma/test:keyless_v2@sha256:2dbc250c79306c30801216e37cd25164c64fda9ac3b9677c5eb0860cb13dbb87"}]} |
+      | POLICY_CONFIGURATION    | ${NAMESPACE}/${POLICY_NAME}                                               |
+      | CERTIFICATE_IDENTITY    | sbaird@redhat.com                                                         |
+      | CERTIFICATE_OIDC_ISSUER | https://accounts.google.com                                               |
+      | REKOR_HOST              | https://rekor.sigstore.dev                                                |

Evidence
The scenario explicitly calls out the external dependency and that the identity is a personal
account, and the helper scripts also bake in that identity/issuer. This is a known maintenance risk
for CI stability.
features/task_validate_image.feature[362-376]
hack/keyless-test-image/verify.sh[24-29]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
Acceptance tests rely on an external quay.io image signed with a personal identity. This is brittle and can cause unrelated CI failures if the image/identity changes.

### Issue Context
The scenario itself acknowledges this risk.

### Fix Focus Areas
- features/task_validate_image.feature[341-381]
- hack/keyless-test-image/create.sh[22-33]
- hack/keyless-test-image/verify.sh[24-33]

### Suggested approach
- Replace the external fixture with a test image created/signed during the test run (if possible), or
- Move the fixture to a project-controlled repository and sign with a stable service identity; document rotation procedure.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

tasks/verify-enterprise-contract/0.1/verify-enterprise-contract.yaml

codecov · 2026-02-28T20:32:32Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

Flag	Coverage Δ
acceptance	`54.84% <ø> (+<0.01%)`	⬆️
generative	`18.16% <ø> (ø)`
integration	`27.00% <ø> (ø)`
unit	`68.65% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

simonbaird · 2026-03-02T19:49:08Z

I added a commit to address Qodo's "action required" about the env vars vs params.

The other two suggestions I think I'm okay with leaving as-is. The partial oidc values I want to handle in ec rather than handle in bash. For the "tests use canned image", it's explain in comments, and I'll file a new Jira to track improving this.

features/__snapshots__/task_validate_image.snap

I'm doing this first in its own separate commit because it will make the next commit easier to understand and review. There's no functional change happening, we're just defining the task step in a different way. Note: - There are some clear inconsistencies in the way we use `--arg=value` or `--arg value` but I'm keeping all that as-is, since I want this change to be as small as possible. - For the same reason I'm keeping the commentary, and the timeout param, which I think could be removed now. Not touching it. - I'm focussing on just the verify-enterprise-contract task only here. My plan is to get that working first, then tackle the other tasks. Ref: https://issues.redhat.com/browse/EC-1652 Co-authored-by: Claude Code <noreply@anthropic.com>

We're keeping the new param handling bash as light as possible. My thinking is that ec should produce a good enough error if the params are wrong or incorrectly specified, so we don't want or need a layer messy bash to check that they are correct. Ref: https://issues.redhat.com/browse/EC-1652 Co-authored-by: Claude Code <noreply@anthropic.com>

simonbaird · 2026-03-03T18:20:01Z

/review

qodo-code-review · 2026-03-03T18:20:08Z

Persistent review updated to latest commit 125d6d0

simonbaird · 2026-03-03T18:20:22Z

I think the acceptance test fail is spurious since it passes locally.

As per the comments, I'm creating an image that we can use with the acceptance tests. Unlike the "golden-container" it wasn't built in Konflux and hence it doesn't have a realistic Chains style attestation. But it does have a keylessly sig and an a valid attestation. And actually there are two images, one created with cosign v2, and one with cosign v3. This should be useful since we need to support the new sigstore bundle introduced in cosign v3. The verify.sh isn't important for anything, but I want to check it in anyhow, since it serves as a nice instruction on how to verify the keyless signed images and attestations, and it's useful to sanity check the images when/if they need recreating. Ref: https://issues.redhat.com/browse/EC-1652

Note that the snapshot output reveals there are some bugs related to to how we output image signatures and attestation signature for the image with the v3 style sigstore bundle. I want to fix this later since this PR already has a lot of changes, so I filed https://issues.redhat.com/browse/EC-1690 to track it. Ref: https://issues.redhat.com/browse/EC-1652 Co-authored-by: Claude Code <noreply@anthropic.com>

simonbaird · 2026-03-03T19:41:29Z

I'm planning to stop tinkering with this. Hopefully that's the last revision/CI run.

simonbaird · 2026-03-03T19:42:21Z

FYI I think the "Tekton script param injection" Qodo complaint is stale.

simonbaird · 2026-03-03T20:03:14Z

/retest

robnester-rh · 2026-03-03T21:08:38Z

/review

qodo-code-review · 2026-03-03T21:09:05Z

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 PR contains tests
🔒 Security concerns Supply chain / unpinned tooling: The scripts under `hack/keyless-test-image/` execute `go run github.com/sigstore/cosign/...@latest`, which pulls and runs remote code at execution time. Even though this is under `hack/`, it is still a sharp edge if someone runs it in automation; consider pinning versions (commit/tag) and/or providing checksums or a containerized toolchain. Let @robnester-rh know what you think of this process.
⚡ Recommended focus areas for review Input Validation The new argument-selection logic prefers keyless options only when both certificate parameters are set, otherwise falls back to public key if present. If a user sets only one of the certificate fields (or sets neither certificate fields nor public key), the task will invoke `ec` without any verification material flags, which may lead to confusing failures or unexpected behavior. Consider failing fast with a clear error message when an incomplete keyless configuration is provided, and/or documenting precedence rules explicitly. script: \| #!/bin/bash set -euo pipefail cmd_args=( "validate" "image" "--images" "/tekton/home/snapshot.json" "--policy" "${POLICY_CONFIGURATION}" ) # To keep bash logic as thin as possible we deliberately don't sanitize # these params. If something is wrong or missing let Conforma handle it. if [ -n "${CERTIFICATE_IDENTITY}" ] && [ -n "${CERTIFICATE_OIDC_ISSUER}" ]; then cmd_args+=( "--certificate-identity" "${CERTIFICATE_IDENTITY}" "--certificate-oidc-issuer" "${CERTIFICATE_OIDC_ISSUER}" ) elif [ -n "${PUBLIC_KEY}" ]; then cmd_args+=( "--public-key" "${PUBLIC_KEY}" ) fi Flag Semantics The `IGNORE_REKOR` flag is now passed as `--ignore-rekor=<value>` instead of `--ignore-rekor <value>` style used previously. Confirm the `ec` CLI accepts the equals-form consistently across supported versions, and that string values like "false"/"true" are interpreted as intended. cmd_args+=( "--rekor-url" "${REKOR_HOST}" "--ignore-rekor=${IGNORE_REKOR}" "--workers" "${WORKERS}" # NOTE: The syntax below is required to negate boolean parameters "--info=${INFO}" # Fresh versions of ec support "--timeout=0" to indicate no timeout, but this would break # the task if it's used with an older version of ec. In an abundance of caution, let's set # an arbitrary high value instead of using 0 here. In future we can change it to 0. # (The reason to not use an explicit timeout for ec is so Tekton can handle the timeouts). "--timeout=100h" "--strict=false" "--show-successes" "--effective-time=${EFFECTIVE_TIME}" Test Stability New acceptance scenarios rely on externally hosted images in `quay.io/conforma/test` pinned by digest. This can still be brittle if the repo is cleaned up, access changes, or the referenced artifacts are removed. Consider mirroring into a more controlled registry/namespace (or adding a fallback), and clearly documenting the lifecycle/ownership of those test artifacts. # See hack/keyless-test-image for how the quay.io/conforma/test:keyless_v2 # and quay.io/conforma/test:keyless_v3 test images where created. It's not # ideal that this test requires an external image, but we already do this # elsewhere, so I guess one more is okay. # Todo: We should be able test this also with an internally built image # similar to how it's done in the "happy day with keyless" scenario in the # validate_image feature. # Confirm we can verify the signatures on a keylessly signed image signed with cosign v2 Scenario: Keyless signing verification cosign v2 style Given a working namespace Given a cluster policy with content: ``` { "sources": [ { "policy": [ "github.com/conforma/policy//policy/release?ref=0de5461c14413484575e63e96ddb514d8ab954b5", "github.com/conforma/policy//policy/lib?ref=0de5461c14413484575e63e96ddb514d8ab954b5" ], "config": { "include": [ "slsa_provenance_available" ] } } ] } ``` When version 0.1 of the task named "verify-enterprise-contract" is run with parameters: \| IMAGES \| {"components": [{"containerImage": "quay.io/conforma/test:keyless_v2@sha256:03a10dff06ae364ef9727d562e7077b135b00c7a978e571c4354519e6d0f23b8"}]} \| \| POLICY_CONFIGURATION \| ${NAMESPACE}/${POLICY_NAME} \| \| CERTIFICATE_IDENTITY \| conformacommunity@gmail.com \| \| CERTIFICATE_OIDC_ISSUER \| https://accounts.google.com \| \| REKOR_HOST \| https://rekor.sigstore.dev \| \| IGNORE_REKOR \| false \| \| STRICT \| true \| Then the task should succeed And the task logs for step "report-json" should match the snapshot And the task results should match the snapshot # Confirm we can verify the signatures on a keylessly signed image signed with cosign v3 Scenario: Keyless signing verification cosign v3 style Given a working namespace Given a cluster policy with content: ``` { "sources": [ { "policy": [ "github.com/conforma/policy//policy/release?ref=0de5461c14413484575e63e96ddb514d8ab954b5", "github.com/conforma/policy//policy/lib?ref=0de5461c14413484575e63e96ddb514d8ab954b5" ], "config": { "include": [ "slsa_provenance_available" ] } } ] } ``` When version 0.1 of the task named "verify-enterprise-contract" is run with parameters: \| IMAGES \| {"components": [{"containerImage": "quay.io/conforma/test:keyless_v3@sha256:712ca3a7fcd41fe6b3e6f434a31f738743b6c31f1d81ad458502d6b0239a8903"}]} \| \| POLICY_CONFIGURATION \| ${NAMESPACE}/${POLICY_NAME} \| \| CERTIFICATE_IDENTITY \| conformacommunity@gmail.com \| \| CERTIFICATE_OIDC_ISSUER \| https://accounts.google.com \| \| REKOR_HOST \| https://rekor.sigstore.dev \| \| IGNORE_REKOR \| false \| \| STRICT \| true \| Then the task should succeed And the task logs for step "report-json" should match the snapshot And the task results should match the snapshot

robnester-rh

LGTM - thanks for doing this.

github-actions bot added the size: XL label Feb 28, 2026

qodo-code-review bot reviewed Feb 28, 2026

View reviewed changes

tasks/verify-enterprise-contract/0.1/verify-enterprise-contract.yaml Show resolved Hide resolved

simonbaird mentioned this pull request Mar 2, 2026

Support Sigstore bundle verification for cosign v3 #3123

Merged

simonbaird force-pushed the task-keyless-support branch from 3f4a73d to 0a2662b Compare March 2, 2026 18:48

github-actions bot added size: XXL and removed size: XL labels Mar 2, 2026

simonbaird changed the title ~~Support validating keyless signed images in verify-enterprise-contract tekton task~~ Support validating keyless signed images in v-e-c tekton task Mar 2, 2026

simonbaird force-pushed the task-keyless-support branch from 0a2662b to 94691c7 Compare March 2, 2026 23:13

simonbaird commented Mar 2, 2026

View reviewed changes

features/__snapshots__/task_validate_image.snap Show resolved Hide resolved

simonbaird commented Mar 2, 2026

View reviewed changes

features/__snapshots__/task_validate_image.snap Show resolved Hide resolved

simonbaird and others added 2 commits March 3, 2026 12:15

simonbaird force-pushed the task-keyless-support branch from 94691c7 to 125d6d0 Compare March 3, 2026 17:32

simonbaird and others added 2 commits March 3, 2026 14:15

simonbaird force-pushed the task-keyless-support branch from 125d6d0 to 63baa34 Compare March 3, 2026 19:39

qodo-code-review bot added Review effort 3/5 Possible security concern labels Mar 3, 2026

robnester-rh approved these changes Mar 3, 2026

View reviewed changes

Conversation

simonbaird commented Feb 28, 2026

Uh oh!

qodo-code-review bot commented Feb 28, 2026

Review Summary by Qodo

Walkthroughs

File Changes

Uh oh!

qodo-code-review bot commented Feb 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Previous review results

Uh oh!

Uh oh!

codecov bot commented Feb 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

simonbaird commented Mar 2, 2026

Uh oh!

Uh oh!

Uh oh!

simonbaird commented Mar 3, 2026

Uh oh!

qodo-code-review bot commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

simonbaird commented Mar 3, 2026

Uh oh!

simonbaird commented Mar 3, 2026

Uh oh!

simonbaird commented Mar 3, 2026

Uh oh!

simonbaird commented Mar 3, 2026

Uh oh!

robnester-rh commented Mar 3, 2026

Uh oh!

qodo-code-review bot commented Mar 3, 2026

PR Reviewer Guide 🔍

Uh oh!

robnester-rh left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

qodo-code-review bot commented Feb 28, 2026 •

edited

Loading

codecov bot commented Feb 28, 2026 •

edited

Loading

qodo-code-review bot commented Mar 3, 2026 •

edited

Loading